-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[AMDGPU][gfx1250] Check for GloballyAddressableScratch in mayAccessScratchThroughFlat #160669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
This is only used by SIInsertWaitCnt, not sure how to test it? There is no dedicated test for https://reviews.llvm.org/D153295 I believe |
|
@llvm/pr-subscribers-backend-amdgpu Author: Pierre van Houtryve (Pierre-vh) ChangesFull diff: https://github.com/llvm/llvm-project/pull/160669.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 044ea866342c2..a98dc3fe7552f 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4319,8 +4319,10 @@ bool SIInstrInfo::mayAccessScratchThroughFlat(const MachineInstr &MI) const {
if (!isFLAT(MI) || isFLATGlobal(MI))
return false;
- // If scratch is not initialized, we can never access it.
- if (MI.getMF()->getFunction().hasFnAttribute("amdgpu-no-flat-scratch-init"))
+ // If scratch is not initialized, we can never access it unless the target has
+ // the ability to access scratch outside its own thread.
+ if (!ST.hasGloballyAddressableScratch() &&
+ MI.getMF()->getFunction().hasFnAttribute("amdgpu-no-flat-scratch-init"))
return false;
// SCRATCH instructions always access scratch.
|
arsenm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testcase?
It was also used in SIMemoryLegalizer but I see you removed that in #157640. Prior to that, mayAccessScratchThroughFlat lived in SIInsertWaitcnts.cpp. For the use in SIInsertWaitcnts I think we only care about accesses to this wave's scratch, so I'm not sure any code change is needed here? |
|
To explain a bit more: in normal wave termination, the hw waits for outstanding stores to complete, and then releases the wave's VGPRs and scratch. If you send MSG_DEALLOC_VGPRS before the end of the shader, then VGPRs and scratch will be released before the hw waits for outstanding stores to complete. If there were any outstanding stores to this wave's scratch then that will cause a problem. |
Ah that makes sense, then indeed no code change is needed here. |

No description provided.